Kiro CLI TUIを共通live E2E policy/lifecycleへ接続する - #2233
Conversation
|
Warning Review limit reached
Next review available in: 20 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (9)
📝 WalkthroughWalkthroughKiro TUIのライブE2E実行を追加しました。認証データとチャットランタイムを参照バインドし、専用tmux環境で実行します。実行ゲート、ジャーニー、クリーンアップ、統合テスト、ローカルE2Eテストを追加しました。 ChangesKiro TUIライブE2E
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/harness/kiro-tui-live.ts`:
- Around line 33-53: kiroTuiLiveSkipReason の live gate に、handleDoctor と共通の Bun
利用可能性および `/amadeus --doctor` の preflight 検査を追加してください。各検査の失敗時は既存の skip reason
形式でゲートを停止し、Codex CLI 0.139.0 の検査は追加しないでください。Bun または `/amadeus --doctor`
が失敗する場合を検証する integration test も追加してください。
In `@tests/harness/live-e2e/journey.ts`:
- Around line 136-163: Extract the duplicated assertion and evidence logic from
createKiroTuiJourney and createClaudeTuiJourney into a shared journey factory.
Parameterize only the differing id, prompt, timeoutMs, diagnostic text, and
evidence.kind values, while preserving the existing five validation conditions
and digest construction consistently for both journeys.
In `@tests/harness/live-e2e/kiro-tui.ts`:
- Around line 463-471: Update `#reapPrivateServer` to distinguish an absent
private server from other list-sessions failures: return null only for the
absentPrivateServer case, and immediately return the command failure’s stderr
for any other failure instead of polling until the deadline. Preserve the
existing timeout message for a server that remains present through the deadline.
In `@tests/harness/live-e2e/tmux.ts`:
- Around line 35-41: Update the spawnSync options in TmuxCommandRunner.run to
include the established 15,000 ms timeout used by `#probeKiro`. Keep the existing
commandFailed and stderr handling unchanged so timeout results with null status
and an error are processed through the current path.
In `@tests/integration/t-live-e2e-kiro-tui.integration.test.ts`:
- Around line 185-189: Update the serverCalls assertion in the integration test
to use the MAX_SOCKET_PATH_BYTES constant exported from kiro-tui.ts instead of
the literal 100, importing that symbol so the test and implementation share the
same limit.
- Around line 302-313: Update the test “a trust picker that never clears reaches
the readiness deadline” to retain the FakePrivateTmux instance and assert its
tmux.calls: confirm the trust-clearing Down command was sent and no prompt
send-keys -l command was issued, while preserving the existing recorded failure
outcome assertion and cleanup.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 8471acc6-b15b-492a-a783-f112a09bff0a
📒 Files selected for processing (13)
docs/harness-engineering/live-e2e.mdtests/e2e/t-kiro-tui-kernel.serial.test.tstests/harness/kiro-tui-live.tstests/harness/live-e2e/claude-tui.tstests/harness/live-e2e/claude.tstests/harness/live-e2e/journey.tstests/harness/live-e2e/kiro-tui.tstests/harness/live-e2e/kiro.tstests/harness/live-e2e/registry.tstests/harness/live-e2e/scratch.tstests/harness/live-e2e/tmux.tstests/integration/t-kiro-tui-live-gate.integration.test.tstests/integration/t-live-e2e-kiro-tui.integration.test.ts
The private tmux mechanics and the scratch git initialisation were private to the Claude adapters, so a second rendered-TUI adapter could not reuse them without importing a Claude module. Move both behind their own modules; claude-tui re-exports the tmux types so existing importers are unchanged.
…ecycle Add KiroTuiAdapter, the kiro-tui capability row, its anchor journey, and the gate helper, so Kiro TUI runs through the same gate, resource registrar, cleanup barrier, and canonical outcome taxonomy as the other transports. Kiro keeps authentication in an on-disk database under the user's home and re-executes its chat runtime from a per-home path, and neither is reachable through an environment variable, so a scratch home cannot even launch the TUI. The scratch home therefore binds those two source entries by reference: no credential bytes are copied into scratch, the adapter never writes to the source home, and removing the scratch tree removes the binding. Two constraints are load-bearing and were measured, not assumed. The run-private socket lives in the temp directory under a short run-identified name because a socket nested inside the scratch root overflows the platform UNIX domain socket path limit. The pane command is resolved to an absolute path because tmux runs it through a shell whose startup files can rewrite PATH.
Fake-tmux integration tests pin the contract normal CI can prove without launching a live process: exact opt-in and CI deny with zero side effects, preflight skip before any spawn, allow-list child environment with no ambient credential or source path, source auth bound by symlink and left unmodified, cleanup failure never reaching the ledger, and a trust picker that never clears reaching the readiness deadline. The serial kernel test runs the real journey behind AMADEUS_KIRO_TUI_LIVE=1 and stays skipped otherwise.
The first live runs left a scratch root behind even though the run reported closed: cleanup removed the tree while the killed child was still shutting down, and the child then recreated files under its scratch HOME after the leak check had already looked. tmux leaves the socket file in place after kill-server, so liveness is now asked of the server itself, and scratch removal is re-verified before the barrier accepts closure. An unreaped server or a reappearing scratch root is a cleanup failure, which forbids a PASS. Also pin the built-in kiro_default agent for the launch. The journey measures the TUI transport, and the shipped conductor is a workflow surface with its own journeys; with the distribution installed the conductor became the workspace default and its turns never completed, timing out a transport that was healthy. Verified with three consecutive live runs: all green, no scratch root, socket, or kiro-cli process left behind.
53c305e to
a05fd87
Compare
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (5)
tests/harness/live-e2e/tmux.ts (1)
35-41: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
spawnSyncにtimeoutを設定してください。
runは同期呼び出しです。tmux クライアントがサーバー応答を待って停止すると、呼び出しスレッドが無期限にブロックします。この間#waitForReadyの deadline もAbortSignalも評価されません。kiro-tui.tsの#probeKiroはspawnSyncにtimeout: 15_000を設定しています。共通ポートにも同じ境界を入れてください。🛡️ 提案する修正
export interface TmuxCommandOptions { readonly cwd?: string; readonly env?: Readonly<Record<string, string>>; + readonly timeoutMs?: number; }const result = spawnSync(this.#tmuxBin, [...args], { cwd: options.cwd, env: options.env, encoding: "utf8", maxBuffer: MAX_PANE_BYTES + 1, + timeout: options.timeoutMs ?? 15_000, });timeout 超過時は
statusがnull、errorが設定されます。commandFailedとstderrの組み立ては既にその形を吸収します。🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/harness/live-e2e/tmux.ts` around lines 35 - 41, Update the spawnSync call in TmuxCommandRunner.run to include a 15,000 ms timeout, matching the existing `#probeKiro` boundary. Preserve the current commandFailed and stderr handling for timeout results.tests/harness/live-e2e/kiro-tui.ts (1)
463-471: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
#reapPrivateServerが恒久的なコマンド失敗を「未 reap」と誤診断します。行 467 はサーバー消滅を
commandFailed(listed) && absentPrivateServer(listed)でのみ判定します。list-sessionsが別の理由で失敗する場合 — ソケットへのアクセス拒否、tmuxバイナリの不在、コマンドタイムアウト — このループは同じ失敗を deadline (既定 10 秒) まで polling します。その後"private tmux server was not reaped"を返し、cleanup barrier が PASS を落とします。サーバーは既に消えている可能性があります。失敗の種類で分岐してください。
absentPrivateServerではない失敗は、そのstderrをそのまま診断として返してください。原因の追跡が可能になります。♻️ 提案する修正
async `#reapPrivateServer`(): Promise<string | null> { const deadline = Date.now() + (this.#options.reapTimeoutMs ?? 10_000); for (;;) { const listed = this.#privateCommand(["list-sessions"]); - if (commandFailed(listed) && absentPrivateServer(listed)) return null; + if (commandFailed(listed)) { + if (absentPrivateServer(listed)) return null; + return sanitizeText(listed.stderr || listed.stdout || "private tmux server liveness is unknown"); + } if (Date.now() >= deadline) return "private tmux server was not reaped"; await this.#sleep(); } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/harness/live-e2e/kiro-tui.ts` around lines 463 - 471, Update `#reapPrivateServer` to distinguish an absent private server from other list-sessions failures: retain the null return for failures matching absentPrivateServer(listed), but immediately return the command’s stderr for any other command failure instead of polling until the deadline. Preserve the existing timeout message for a successful command that never confirms reaping.tests/harness/live-e2e/journey.ts (1)
136-163: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
createClaudeTuiJourneyとの完全重複が残っています。共通ファクトリへ抽出してください。行 143-147 の 5 項判定と行 155-157 の digest 式は行 107-134 と同一です。差分は
id、prompt、timeoutMs、診断文、evidence.kindのみです。判定条件を片方だけに追加するドリフトが発生します。♻️ 提案するリファクタ
+interface TuiAnchorJourneySpec { + readonly id: string; + readonly prompt: string; + readonly timeoutMs: number; + readonly evidenceKind: string; + readonly passedDiagnostic: string; + readonly failedDiagnostic: string; +} + +function createTuiAnchorJourney(spec: TuiAnchorJourneySpec): LiveJourney { + return { + id: spec.id, + prompt: spec.prompt, + timeoutMs: spec.timeoutMs, + retryPolicy: { maxAttempts: 1 }, + assert: (execution) => { + const passed = execution.exitCode === 0 && + execution.structured?.anchorVerified === true && + execution.structured.inputCount === 1 && + typeof execution.structured.paneDigest === "string" && + typeof execution.structured.sessionDigest === "string"; + return { + passed, + diagnostic: passed ? spec.passedDiagnostic : spec.failedDiagnostic, + evidence: [{ + kind: spec.evidenceKind, + value: digest( + `${execution.exitCode}:${execution.structured?.anchorVerified}:${execution.structured?.inputCount}:${execution.structured?.paneDigest}`, + ), + source: "assertion", + }], + }; + }, + }; +} + export function createKiroTuiJourney(): LiveJourney { - return { - id: "kiro-tui-anchor-v1", - prompt: KIRO_TUI_PROMPT, - timeoutMs: 180_000, - retryPolicy: { maxAttempts: 1 }, - assert: (execution) => { - const passed = execution.exitCode === 0 && - execution.structured?.anchorVerified === true && - execution.structured.inputCount === 1 && - typeof execution.structured.paneDigest === "string" && - typeof execution.structured.sessionDigest === "string"; - return { - passed, - diagnostic: passed - ? "private Kiro TUI session, current-run file anchor, and bounded pane evidence passed" - : "Kiro TUI anchor mismatch", - evidence: [{ - kind: "kiro-tui-anchor", - value: digest( - `${execution.exitCode}:${execution.structured?.anchorVerified}:${execution.structured?.inputCount}:${execution.structured?.paneDigest}`, - ), - source: "assertion", - }], - }; - }, - }; + return createTuiAnchorJourney({ + id: "kiro-tui-anchor-v1", + prompt: KIRO_TUI_PROMPT, + timeoutMs: 180_000, + evidenceKind: "kiro-tui-anchor", + passedDiagnostic: + "private Kiro TUI session, current-run file anchor, and bounded pane evidence passed", + failedDiagnostic: "Kiro TUI anchor mismatch", + }); }
createClaudeTuiJourneyも同じファクトリへ載せてください。🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/harness/live-e2e/journey.ts` around lines 136 - 163, Extract the duplicated assertion and evidence logic from createClaudeTuiJourney and createKiroTuiJourney into a shared journey factory. Parameterize only the differing id, prompt, timeoutMs, diagnostic text, and evidence kind, then have both creators use that factory so their five-part validation and digest construction remain identical.tests/integration/t-live-e2e-kiro-tui.integration.test.ts (2)
185-189: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
100はマジックナンバーです。MAX_SOCKET_PATH_BYTESを参照してください。
tests/harness/live-e2e/kiro-tui.tsが同じ値をMAX_SOCKET_PATH_BYTESとして持ちます。テストが独立した定数を持つと、片方の変更でドリフトします。kiro-tui.tsから export して import してください。🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/integration/t-live-e2e-kiro-tui.integration.test.ts` around lines 185 - 189, テストのソケットパス長検証にあるマジックナンバー100を削除し、tests/harness/live-e2e/kiro-tui.tsのMAX_SOCKET_PATH_BYTESをexportして、この統合テストへimportし参照してください。既存のserverCalls検証条件は維持してください。
302-313: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winこのテストは readiness deadline への到達を証明していません。
AMADEUS_LIVE_E2E:FAIL:EXECUTION_FAILEDは他の失敗経路でも返ります。tmux コマンド失敗、pane 上限超過、anchor 不一致でも同じコードです。診断文はstderrDigestとしてハッシュ化されるので、受領証から readiness 失敗を区別できません。
tmux.callsを検証してください。trust クリアのEnterが送られ、prompt のsend-keys -lが送られていないことを示せば、readiness 段階で停止したと確定できます。💚 提案する検証の追加
test("a trust picker that never clears reaches the readiness deadline", async () => { const item = fixture(); + const tmux = new FakePrivateTmux({ stuckOnTrustPrompt: true }); try { - const result = await runFixture(item, new FakePrivateTmux({ stuckOnTrustPrompt: true }), 0); + const result = await runFixture(item, tmux, 0); expect(result).toMatchObject({ ok: true, value: { kind: "recorded", outcome: { code: "AMADEUS_LIVE_E2E:FAIL:EXECUTION_FAILED" } }, }); + // Readiness never completed, so the prompt was never sent. + expect(tmux.calls.some((call) => call.args[2] === "send-keys")).toBe(true); + expect(tmux.calls.some((call) => call.args.includes("-l"))).toBe(false); } finally {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/integration/t-live-e2e-kiro-tui.integration.test.ts` around lines 302 - 313, Strengthen the test “a trust picker that never clears reaches the readiness deadline” by inspecting the FakePrivateTmux instance’s `calls` after `runFixture` completes. Assert that the trust-clearing Enter was sent and that no prompt `send-keys -l` command was issued, while preserving the existing recorded failure-code assertion and cleanup.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/harness/live-e2e/kiro.ts`:
- Around line 57-67: Update kiroHomeLayout to accept the environment needed for
path resolution and, for non-darwin platforms, use XDG_DATA_HOME when set before
falling back to ~/.local/share. Ensure callers such as the kiro-tui registry
path checks pass the relevant environment so authFile matches the actual data
directory.
In `@tests/harness/live-e2e/scratch.ts`:
- Around line 25-37: Update the git failure diagnostic in the loop around
spawnSync to report the actual subcommand name explicitly for each argument set,
especially “commit,” instead of deriving it from args[0]. Preserve the existing
sanitized stderr and failure behavior.
In `@tests/integration/t-kiro-tui-live-gate.integration.test.ts`:
- Around line 108-114: Extend the negative-case loop for
createKiroTuiJourney().assert to independently invalidate all five passed
conditions: exitCode, anchorVerified, inputCount, paneDigest type, and
sessionDigest type. Keep the existing anchorVerified and inputCount cases, add
cases for the remaining three fields, and assert passed is false for each so
removing any condition from the predicate causes the test to fail.
---
Duplicate comments:
In `@tests/harness/live-e2e/journey.ts`:
- Around line 136-163: Extract the duplicated assertion and evidence logic from
createClaudeTuiJourney and createKiroTuiJourney into a shared journey factory.
Parameterize only the differing id, prompt, timeoutMs, diagnostic text, and
evidence kind, then have both creators use that factory so their five-part
validation and digest construction remain identical.
In `@tests/harness/live-e2e/kiro-tui.ts`:
- Around line 463-471: Update `#reapPrivateServer` to distinguish an absent
private server from other list-sessions failures: retain the null return for
failures matching absentPrivateServer(listed), but immediately return the
command’s stderr for any other command failure instead of polling until the
deadline. Preserve the existing timeout message for a successful command that
never confirms reaping.
In `@tests/harness/live-e2e/tmux.ts`:
- Around line 35-41: Update the spawnSync call in TmuxCommandRunner.run to
include a 15,000 ms timeout, matching the existing `#probeKiro` boundary. Preserve
the current commandFailed and stderr handling for timeout results.
In `@tests/integration/t-live-e2e-kiro-tui.integration.test.ts`:
- Around line 185-189:
テストのソケットパス長検証にあるマジックナンバー100を削除し、tests/harness/live-e2e/kiro-tui.tsのMAX_SOCKET_PATH_BYTESをexportして、この統合テストへimportし参照してください。既存のserverCalls検証条件は維持してください。
- Around line 302-313: Strengthen the test “a trust picker that never clears
reaches the readiness deadline” by inspecting the FakePrivateTmux instance’s
`calls` after `runFixture` completes. Assert that the trust-clearing Enter was
sent and that no prompt `send-keys -l` command was issued, while preserving the
existing recorded failure-code assertion and cleanup.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 14989e7f-10d5-4a26-b524-216e6f481857
⛔ Files ignored due to path filters (2)
amadeus/spaces/default/intents/260804-live-e2e-phase2/amadeus-state.mdis excluded by!amadeus/spaces/*/intents/**amadeus/spaces/default/intents/260804-live-e2e-phase2/audit/j5ik2o-mac-studio-lan-da10ea4c23f8.jsonlis excluded by!amadeus/spaces/*/intents/**
📒 Files selected for processing (15)
docs/harness-engineering/live-e2e.mdtests/e2e/t-kiro-tui-kernel.serial.test.tstests/harness/kiro-tui-live.tstests/harness/live-e2e/claude-tui.tstests/harness/live-e2e/claude.tstests/harness/live-e2e/journey.tstests/harness/live-e2e/kiro-tui.tstests/harness/live-e2e/kiro.tstests/harness/live-e2e/registry.tstests/harness/live-e2e/scratch.tstests/harness/live-e2e/tmux.tstests/integration/t-kiro-tui-live-gate.integration.test.tstests/integration/t-live-e2e-kiro-tui.integration.test.tstests/no-silent-drop/baseline.jsontests/no-silent-drop/exemptions.json
- bound every tmux command with a spawnSync timeout so a stalled client cannot outlive the journey deadline - make #reapPrivateServer report non-absent list-sessions failures verbatim instead of polling them to the deadline - extract the shared TUI anchor journey factory to stop assert drift between the Claude and Kiro journeys - share MAX_SOCKET_PATH_BYTES with the integration test and pin the readiness-deadline test to the readiness path
- resolve the source-side Kiro data directory through XDG_DATA_HOME on Linux, matching the registry's sourcePathKeys declaration, while scratch homes keep their pure home-offset layout - name the git subcommand in scratch init failure diagnostics - negate every term of the TUI anchor predicate in the gate test
Line shifts pulled the multi-line return-type union of #readinessStep and the defaultKiroSourceHome signature into the patch diff, where bun lcov stamps type-only continuation lines DA:0 under union merge. A module-scope ReadinessObservation alias and a one-line signature keep every measured patch line an executable one.
Every existing consumer passes an explicit sourceHome, so the helper never ran under coverage and its declaration line stayed uncovered in the patch gate. The direct test also fixes the env-override contract.
概要
Issue #1717 Phase 2 / live-e2e-phase2 intent の Bolt 1(Walking Skeleton)。Kiro CLI TUI を共通 live E2E policy/lifecycle へ direct branch で接続します。
KiroTuiAdapter(tests/harness/live-e2e/kiro-tui.ts)+ Kiro auth/config binding(kiro.ts): scratch HOME への symlink 束縛(Kimi precedent)。資格情報バイトは source home の外へ出さず、scratch tree 削除で束縛全体が消えるtmux.ts/scratch.tsへ抽出し共有(公開 export 不変、claude-tui 回帰 12 pass)kiro-tui行 +createKiroTuiJourney+ opt-in live gate(AMADEUS_KIRO_TUI_LIVE=1)。通常 CI は live process を起動しない実測で発見・修正した欠陥
設計からの逸脱(batch ゲートで承認済み)
--agent kiro_defaultを固定(測定対象を TUI transport に限定。conductor-on-Kiro は既存 journey が担当)検証
Related to #1717
Intent mirror: #2173
Note
Medium Risk
Touches test harness isolation, tmux cleanup barriers, and symlink binding to real user Kiro auth; mistakes could leak paths/credentials or leave processes/sockets behind, though CI stays hard-denied and coverage is heavy on fakes.
Overview
Adds
kiro-tuito the shared live E2E harness: registry row, opt-in gate (AMADEUS_KIRO_TUI_LIVE=1),createKiroTuiJourney, serial kernel test, and runbook/matrix updates.KiroTuiAdapterdriveskiro-cli chatin a run-private tmux session (short socket under system temp, trust-picker handling, run-bound file anchor, bounded pane digests). Auth is not an env lease:KiroHomeCredentialSourceandbindKiroScratchHomesymlink the source home’s auth DB and chat runtime into scratch without copying or mutating the user home.Refactors Claude TUI helpers into shared
tmux.tsandscratch.ts(initializeScratchGit); Claude scratch allocators call the shared git init.Integration tests cover gates, symlink binding, cleanup barrier (reap + scratch re-removal), and evidence that secrets/paths do not leak into receipts.
Reviewed by Cursor Bugbot for commit 10c73b6. Configure here.
Summary by CodeRabbit
新機能
ドキュメント
テスト